Refactor about:preferences#payments followups #6047

luixxiul · 2016-12-06T17:02:10Z

Submitted a ticket for my issue if one did not already exist.
Used Github auto-closing keywords in the commit message.
Added/updated tests for this change (for new code or code which already has tests).
Ran git rebase -i to squash commits (if needed).

This is a follow-up of #6009

This PR has 6 commits:

Polishment of titleBar and walletBar
Made the selectors specific on about:preferences#payments to avoid unexpected regressions
Added margin-bottom of the transfer funds button (Redo Added margin-bottom to the transfer funds button on about:preferences#payments #6049)
Fixed advanced settings modal dialogs on about:preferences#payments
Polishment of modal dialog and advanced settings dialog on about:preferences#payments
Restyled payment history table

Test plans are explained below.

1st commit

Polishment of titleBar and walletBar

On about:preferences#payments

Disabled underline of the anchor link of fundsFAQ (fixes Funds FAQ icon is underlined on hover #6083)
Set font-size of the elements to 14.5px
Set margin of the elements to 12px
Set min-width to the select, input, and button
Set padding, based on the margin of the items inside it

On about:preferences

Made button size consistent by replacing em with rem (to avoid font-size inconsistency of the buttons inside walletBar) (fixes Buttons on about:payments are thinner than those on other about pages #6216)
Set the default margin of the buttons to 0

Test Plan 1

Open about:preferences
Make sure the style of about:preferences looks consistent

2nd commit

Made the selectors specific on about:preferences#payments to avoid unexpected regressions

Classes and ids were renamed (fixes the buttons on about:preferences#payments need more paddings #6081)
Classes were moved under .paymentsContainer, .bitcoinDashboard, and .ledgerTable
Some elements and classes were removed
Moved elements above classes/ids
Moved .verified out of anchor link (the verified icon is no longer clickable) (fix verified icon on the ledger table should be outside of the anchor link to the site #6225)

See the commit message for more details

Also .paymentHistoryFooter was introduced to fix #6061

Test Plan 2

Make sure all of the selectors listed up in the commit message (except removed ones) can be actually found in the code, especially preferences.js
Make sure the style of about:preferences#payments is not broken
- Open about:preferences#payments
- Toggle off/on switch
- Click "Advanced Settings..."
- Click "Backup your wallet" and "Done"
- Click "Recover your wallet" and "Done"
- Click "Add funds..."
- Click "Display QR code"
Make sure the verified icon is no longer clickable

3rd commit

Added margin-bottom of the transfer funds button (Redo #6049)
This fixes #6048

Test Plan 3

Open https://jsfiddle.net/LnwtLckc/5/
Click "register"
Make sure "Transfer BTC" button has the margin-bottom on add funds dialog on about:preferences#payments

4th commit

This commit will be squashed with the 5th commit after review

Fixed advanced settings modal dialogs on about:preferences#payments

Added white-space:nowrap to recovery keys (fixes #6220)

Test Plan 4

(n/a; covered by Test Plan 5)

5th commit

Polishment of modal dialog and advanced settings dialog on about:preferences#payments

Mockup images by @bradleyrichter: #5719 (comment)

Restyled modal dialog under .paymentsContainer

Made the background of modal dialog consistent
Changed border-radius of modal dialog (fixes border-radius of modal dialog in .paymentsContainer should be 8px #6223)
Fixed spacing of the modal dialog header (fixes Title bar should not have strange spacing in general (on about:preferences#payments) #6221)
Set margin-top/margin-bottom to panel inside .board
Aligned buttons in the footer with flex-end (fixes The buttons on the footers of modal dialogs of payments need to be aligned to right #6219)
Aligned fa icons on the payment panel with display:flex (fixes fa-mobile should be aligned at the center #6222)
Fixed panelFooter in .addFundsBoard (fixes Addfunds dialog needs a little margin #5799)
- Aligned Coinbase icon and message
- Display the footer

For US

For non-US

Restyled advanced settings dialog

Designed "hide sites if" options on the dialog (closes Design "hide sites if" options on Advanced Options Dialog #6201, which is a follow-up of Add "hide sites if" options to Advanced Options Dialog #4681)
Made the margin consistent between the headers/labels and the input/select elements
- Addresses Refactor about:preferences#payments followups #6047 (comment)

Test Plan 5

For "Restyled modal dialog under .paymentsContainer"

Enable Coinbase if not
Open about:preferences#payments
Click "Add funds..."
Make sure the board is well styled
Make sure the footer is aligned horizontally
Click "Fund with debit/credit"
Make sure Coinbase widget is displayed with transparent background
Open https://jsfiddle.net/LnwtLckc/5/
Click "register" and go back
Make sure "Transfer BTC" button is aligned to right

For "Restyled advanced settings dialog"

Click "Advanced Settings..."
Check the margin between the labels and the select forms
Click "Backup your wallet"
Check the margin between the labels and the keys
Click "Done" and "Recover your wallet"
Check the margin between the labels and the input forms

6th commit

Restyled payment history table

Closes #6202
Closes #3485

Removed "pull-right" class from the close button in modalOverlay.less (to confirm 21ece29 of Refactor about:preferences#payments #6009 fixed Close button is not aligned with text in payment history modal #5719)
Added color:@braveMediumOrange to .sectionTitle of .paymentHistory (fixes "Your Payment History" should be orange #6229)
Added padding-left and padding-right to th and td (fixes border-bottom of the table header of the payment history should be extended #6231)
Added white-space:nowrap to disable wrap (fixes receipt links should not be wrapped #6230)
Added min-width:80px to the button inside the footer (fixes OK button should be larger on Payment History dialog #6232)
Made sectionTitle consistent
- Removed ":not(.paymentHistory)" from modalOverlay.less

Test Plan:

Disable the ledger
run npm run add-simulated-payment-history some times
Enable the ledger
Click "View Payment History..."
Click "OK" button

luixxiul · 2016-12-06T17:07:39Z

less/about/preferences.less

@@ -1134,17 +1144,6 @@ table.sortableTable {
      }
    }

-    span {


Note: this is outside of .walletBar. The commit message of 6c168c7 will be fixed when squashing.

luixxiul · 2016-12-06T18:18:55Z

#paymentHistory was replaced with .paymentHistoryTable (because .paymentHistory already exists).

luixxiul · 2016-12-07T03:38:52Z

less/about/preferences.less

+          padding-top: 10px;
+          padding-bottom: 10px;
+
+          .paymentHistoryFooter {


This will fix #6061

bsclifton · 2016-12-09T06:35:48Z

@luixxiul if you want to squash it up, I'm finally checking it out now 😄

I'm not sure how many more UI changes we should pull in, so I'm leaving it as 0.13.1 for now... Will leave notes here after I get a chance to try it 😄

bsclifton · 2016-12-09T06:36:36Z

Actually, with how bad #6061 looks, maybe this should be pulled in

bsclifton

Lots of small changes; I tried it out and I really like it 😄 However, I was curious if you could summarize all of the changes in the original post of the PR. As a reviewer, I'm having a hard time knowing what to look for and I feel that I may make a mistake

bsclifton · 2016-12-09T06:55:22Z

For example, I see the title over the select boxes is very close, almost touching:

Because of the problems we've had with our less being fragile (ex: the selectors not being specific enough), I'd have to try master and compare to know 100% if this changed or not

luixxiul · 2016-12-09T07:03:31Z

@bsclifton thanks, I've noticed it. Should I include the fix into this PR? It will be like this: https://github.com/luixxiul/browser-laptop/commit/2327e9eca53eb4a637dd4c3e3a2a1d1f613c2fbc#commitcomment-19937858

luixxiul · 2016-12-09T07:09:07Z

@bsclifton rather should I add comments on lines https://github.com/brave/browser-laptop/pull/6047/files (or in each commit) to clarify what I changed?

bsclifton · 2016-12-09T08:03:35Z

@luixxiul the change you made above, let's definitely include with this PR 👍

The screenshots are ok- it would help to have a quick summary like:

Payments > Advanced settings: moved up text above select fields (it was too close)
Recovery keys will no longer wrap around
etc

Actually- looking at your commit history, those messages are almost perfect 😄 The pictures are only useful (to me as a reviewer at least) if there is a before and after

luixxiul · 2016-12-10T05:57:52Z

@bsclifton Updated and squashed! I added the summary and the QA steps for each commit in the original post of this PR.

luixxiul · 2016-12-10T11:22:46Z

Updated, removing input[type='range'] (as it exists in sortable.less)

bsclifton · 2016-12-15T06:02:15Z

Big ++ from me too! Really nice cleanup of the original post, BTW 😄

This was an intimidating pull request for sure- but the work is very much appreciated. Going forward, I don't think we'll ever have this many tweaks to the same pages again

Squash it up and then I'll give it one last test and can merge after that 😄
(edit: I am also wanting to make sure automated tests still look good)

@bradleyrichter

…erences#payments Mockup images by @bradleyrichter: 5719#issuecomment-262431933 Restyled modal dialog under .paymentsContainer - Made the background of modal dialog consistent (except .coinbaseOverlay) - Changed border-radius of modal dialog in .paymentsContainer from 4px to 8px (fixes 6223) - Fixed spacing of the modal dialog header (fixes 6221) - Set padding:25px to .sectionTitle in .dialog-header - Set margin-top:0 to the first panel inside .board - Set margin-bottom:0 to the last panel inside .board - Aligned buttons in the footer with flex-end (fixes 6219) - Introduced .addFundsBoard - Aligned fa icons on the payment panel with display:flex (fixes 6222) - Aligned "Transfer BTC button" with display:flex - Fixed panelFooter in .addFundsBoard (fixes 5799) - Aligned Coinbase icon and message - Display the footer (closes 6007) Restyled advanced settings dialog - Designed "hide sites if" options on the dialog (closes 6201, which is a follow-up of 4681) - Made the margin consistent between the headers/labels and the input/select element - Addresses 6047#issuecomment-265946727 Auditors: @bsclifton Test Plan 1 1. Enable Coinbase if not 2. Open about:preferences#payments 3. Click "Add funds..." 4. Make sure the board is well styled 5. Make sure the footer is aligned horizontally 6. Click "Fund with debit/credit" 7. Make sure Coinbase widget is displayed with transparent background 8. Open https://jsfiddle.net/LnwtLckc/5/ 9. Click "register" and go back 10. Make sure "Transfer BTC" button is aligned to right Test Plan 2 1. Click "Advanced Settings..." 2. Check the margin between the labels and the select forms 3. Click "Backup your wallet" 4. Check the margin between the labels and the keys 5. Click "Done" and "Recover your wallet" 6. Check the margin between the labels and the input forms

@mrose17

Closes 6202 Closes 3485 /cc: @mrose17 - Removed "pull-right" class from the close button in modalOverlay.less (to confirm 21ece29 of 6009 fixed 5719) - Added color:@braveMediumOrange to .sectionTitle of .paymentHistory (closes 6229) - Added padding-left and padding-right to th and td (fixes 6231) - Added white-space:nowrap to disable wrap (fixes 6230) - Added min-width:80px to the button inside the footer (fixes 6232) - Made sectionTitle consistent - Removed ":not(.paymentHistory)" from modalOverlay.less Test Plan: 1. Disable the ledger 2. run "npm run add-simulated-payment-history" some times 3. Enable the ledger 4. Click "View Payment History..." 5. Click "OK" button

bsclifton · 2016-12-15T19:08:11Z

Looks great, merging! 😄

@bsclifton

Resolves #7501 #7380 #6364 Auditors: @bsclifton @cezaraugusto Test plan: - everything should work the same as was before the chage

luixxiul added feature/rewards polish Nice to have — usually related to front-end/visual tasks. refactoring labels Dec 6, 2016

luixxiul assigned bsclifton Dec 6, 2016

luixxiul commented Dec 6, 2016

View reviewed changes

bsclifton added this to the 0.13.1 milestone Dec 6, 2016

luixxiul commented Dec 7, 2016

View reviewed changes

less/about/preferences.less

padding-top: 10px;

padding-bottom: 10px;

.paymentHistoryFooter {

Copy link

Contributor Author

luixxiul Dec 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will fix #6061

luixxiul mentioned this pull request Dec 8, 2016

View payment history link needs padding #4867

Closed

luixxiul requested a review from bsclifton December 8, 2016 19:00

bsclifton modified the milestones: 0.13.0, 0.13.1 Dec 9, 2016

bsclifton reviewed Dec 9, 2016

View reviewed changes

luixxiul mentioned this pull request Dec 10, 2016

Added margin-bottom to the transfer funds button on about:preferences#payments #6049

Merged

4 tasks

luixxiul added the QA/test-plan-specified label Dec 10, 2016

luixxiul mentioned this pull request Dec 10, 2016

Refactor about:preferences#payments #6009

Merged

4 tasks

This was referenced Dec 11, 2016

Restyled select and input to the spec image #6084

Closed

Add "hide sites if" options to Advanced Options Dialog #4681

Closed

Font-weight was globaly set to 400 #6149

Closed

the buttons on about:preferences#payments need more paddings #6081

Closed

Suguru Hirahara added 2 commits December 16, 2016 00:59

bsclifton merged commit 8ef38d9 into brave:master Dec 15, 2016

luixxiul removed the release/not-blocking label Dec 15, 2016

This was referenced Dec 16, 2016

Ledger recovery should be aware of network status and give appropriate errors when the connection is down. #4720

Closed

"Advanced Settings..." button on about:preferences#payments should be wider (l10n) #6285

Closed

This was referenced Dec 20, 2016

Manual tests for Windows ia-32 0.13.0 Preview 6 #6335

Closed

Manual tests for Linux Mint 0.13.0 Preview 6 #6337

Closed

Manual tests for Windows x64 0.13.0 Preview 6 #6334

Closed

Manual tests for OS X 0.13.0 Preview 6 #6332

Closed

luixxiul added the QA/checked-Win32 label Dec 20, 2016

luixxiul mentioned this pull request Dec 20, 2016

Buttons not aligned properly in payments advance settings in different language #5771

Closed

luixxiul unassigned jkup and bsclifton Jan 12, 2017

This was referenced Jan 16, 2017

l10n of ledger advanced settings modal window #4786

Closed

Remove min-width for OK button on payment history dialog #6685

Closed

luixxiul removed QA/checked-Win32 QA/test-plan-specified labels Jan 16, 2017

luixxiul mentioned this pull request Jan 30, 2017

Remove outline from addPublisher urlbar button #6761

Merged

luixxiul referenced this pull request Mar 25, 2017

Refactor of paymentTab (#7481)

0bfceeb

Resolves #7501 #7380 #6364 Auditors: @bsclifton @cezaraugusto Test plan: - everything should work the same as was before the chage

This was referenced Apr 2, 2017

Refactor payment/history.js #8037

Closed

Refactor payment/history.js #8038

Merged

luixxiul mentioned this pull request May 5, 2017

Remove paymentStyles.margin.bar, margin.barItem, padding.bar #8710

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor about:preferences#payments followups #6047

Refactor about:preferences#payments followups #6047

luixxiul commented Dec 6, 2016 •

edited

Loading

luixxiul Dec 6, 2016 •

edited

Loading

luixxiul commented Dec 6, 2016 •

edited

Loading

luixxiul Dec 7, 2016

bsclifton commented Dec 9, 2016

bsclifton commented Dec 9, 2016

bsclifton left a comment

bsclifton commented Dec 9, 2016 •

edited

Loading

luixxiul commented Dec 9, 2016 •

edited

Loading

luixxiul commented Dec 9, 2016 •

edited

Loading

bsclifton commented Dec 9, 2016 •

edited

Loading

luixxiul commented Dec 10, 2016 •

edited

Loading

luixxiul commented Dec 10, 2016

bsclifton commented Dec 15, 2016 •

edited

Loading

bsclifton commented Dec 15, 2016

@@ @@ -1134,17 +1144,6 @@ table.sortableTable { @@
                     }
                   }
-                  span {

Refactor about:preferences#payments followups #6047

Refactor about:preferences#payments followups #6047

Conversation

luixxiul commented Dec 6, 2016 • edited Loading

1st commit

Test Plan 1

2nd commit

Test Plan 2

3rd commit

Test Plan 3

4th commit

Test Plan 4

5th commit

Test Plan 5

6th commit

luixxiul Dec 6, 2016 • edited Loading

Choose a reason for hiding this comment

luixxiul commented Dec 6, 2016 • edited Loading

luixxiul Dec 7, 2016

Choose a reason for hiding this comment

bsclifton commented Dec 9, 2016

bsclifton commented Dec 9, 2016

bsclifton left a comment

Choose a reason for hiding this comment

bsclifton commented Dec 9, 2016 • edited Loading

luixxiul commented Dec 9, 2016 • edited Loading

luixxiul commented Dec 9, 2016 • edited Loading

bsclifton commented Dec 9, 2016 • edited Loading

luixxiul commented Dec 10, 2016 • edited Loading

luixxiul commented Dec 10, 2016

bsclifton commented Dec 15, 2016 • edited Loading

bsclifton commented Dec 15, 2016

luixxiul commented Dec 6, 2016 •

edited

Loading

luixxiul Dec 6, 2016 •

edited

Loading

luixxiul commented Dec 6, 2016 •

edited

Loading

bsclifton commented Dec 9, 2016 •

edited

Loading

luixxiul commented Dec 9, 2016 •

edited

Loading

luixxiul commented Dec 9, 2016 •

edited

Loading

bsclifton commented Dec 9, 2016 •

edited

Loading

luixxiul commented Dec 10, 2016 •

edited

Loading

bsclifton commented Dec 15, 2016 •

edited

Loading